Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(sitemap): Add a new configuration to discard invalid patterns from sitemap and avoid duplicate content #176

Merged
merged 4 commits into from
Aug 3, 2024

Conversation

Tyki
Copy link

@Tyki Tyki commented Jul 11, 2024

fix #175

What does it do?

Introduce a new configuration to discard invalid replacement pattern

Why is it needed?

It creates duplicate content if the field is not mandatory

How to test it?

  1. Add a "slug" field to allowedFields
  2. Create a content in which you dont fill the "slug" property in Strapi (for whatever reason)
  3. Generate the sitemap
  4. The sitemap generate multiple entries with the same url because of empty slugs

@Tyki
Copy link
Author

Tyki commented Jul 15, 2024

Hello @boazpoolman, can you review this PR ? We live with a patch-package and want to get rid of it as soon as possible 🙏

@boazpoolman
Copy link
Member

Hi @Tyki

I will make sure to give this a look somewhere this week!

@boazpoolman
Copy link
Member

Hi @Tyki ,

Thanks for reporting this issue and writing a PR to address it!

Though I don't think this is the best approach to solve the problem.
I think when there are two individual pages, there should be two URLs in your sitemap.
After all, these pages should both be accessible in your front-end, hence they should have two unique URLs.

The approach our Webtools plugin takes, is to make sure each page has a different URL, by adding -0, -1, -2 etc. at the end of duplicate URLs. But seeing as I'm currently migrating the Sitemap plugin to be part of the Webtools Suite we don't need to implement that here.

We can release your PR as a workaround that won't be ported back to Webtools.
Could you only make sure the tests pass? I'll be sure to merge and release this afterwards.

@boazpoolman boazpoolman added the Needs work A PR that needs to be modified label Jul 18, 2024
@Tyki
Copy link
Author

Tyki commented Jul 22, 2024

Hi, thanks for the answer

I fixed the tests, let me know if you need anything else in this PR 🙏

@boazpoolman boazpoolman added Needs review A PR that needs to be reviewed and removed Needs work A PR that needs to be modified labels Jul 22, 2024
@boazpoolman
Copy link
Member

Looks good.

Thanks @Tyki.

I'll release this asap.

@boazpoolman boazpoolman merged commit e5a3c04 into pluginpal:master Aug 3, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs review A PR that needs to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

This plugin might generate duplicate content if relational field is not mandatory
2 participants